-
Notifications
You must be signed in to change notification settings - Fork 709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Applied MantisBT #8957: Date Selector for Custom Fields #65
Conversation
Moved script block after rendered calendar icon. As long as the initially needed calendar imports are between the text input and the calendar icon, the first occurrence of the icon is slightly misplaced. Fixed some additional issues related to the JS date picker. See http://www.mantisbt.org/bugs/view.php?id=8957#c33304 for details why I introduced these changes.
* go to http://www.php.net/manual/en/function.date.php | ||
* for detailed instructions on date formatting | ||
* @global string $g_calendar_js_date_format | ||
*/ | ||
$g_calendar_js_date_format = '\%Y-\%m-\%d \%H:\%M'; | ||
$g_calendar_js_date_format = '\%Y-\%m-\%d'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While technically this is correct, it will break for people which have made customisations. I would prefer to keep this unchanged, perhaps use
- $g_calendar_js_date_format
- $g_calendar_js_date_format_long
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, thanks. I didn't think about backwards compatibility too much.
But your naming proposal is also confusing - "long" implies the old setting, so it would also break backwards compatibility by changing the name.
Probably something like the following could be acceptable:
- $g_calendar_js_date_format (for the current setting, so it stays untouched)
- $g_calendar_js_date_no_time_format (for the new one where you don't want to see a time part)
Should be ok then...
…le might already have overridden the old variable) Related discussion: #65
* for detailed instructions on date formatting | ||
* @global string $g_calendar_js_date_no_time_format | ||
*/ | ||
$g_calendar_js_date_no_time_format = '\%Y-\%m-\%d'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the old valiable name stays with its old behaviour. I renamed the new one and edited the comments.
Possible problems with backward compatibility should now be fixed. |
All data selectors having been updated. Hence, closing this issue. |
Follow-up pull request with some more changes.
Moved script block after rendered calendar icon.
As long as the initially needed calendar imports are between the text input and the calendar icon, the first occurrence of the icon is slightly misplaced.
Fixed some additional issues related to the JS date picker.
See http://www.mantisbt.org/bugs/view.php?id=8957#c33304 for details why I introduced these changes.